Options:

1. Return a subset of the metadata that only has relevant information (name, organism, etc). The example listed here is only for reference
2. Return the full metadata.

strongly prefer 1) and having a verbose argument

I created a live corpus unit test for this method. This should ensure that this parsing method remains consistent across releases.

this code has some lint - please run (an up to date) pre-commit across it

Note: this will cause the census object to be re-opened. While this shouldn't be an issue, it will result into an extra call. With some effort I can refactor get_embedding to also accept an existing Census object, but I'm not sure if it's worth it.

IMHO, you should refactor the code to have a (common, shared) function that accepts an already open Census handle

Done.

Given the (high) cost of calling query.to_anndata(), you should do all error checking before the costly ops - ie., move this kind of stuff to a prologue

Good point.

shouldn't these use urljoin()?

this error check (name collision) is missing from the var axis. Seems like you need it for both

I removed varm_layers from the arguments, as you suggested, since it is not in the current API. The only way you can request a varm is through add_varm_embeddings

Right - gotcha!

So, I'm thinking about this previous conversation a bit more. Coming around to a slightly different perspective. Given:

• tiledbsoma/somacore support more functionality than is exposed here (e.g., varp_layers, etc)
• Census doesn't currently use these, due to schema definition (not code)
• we want to clearly separate the arg names for clarity (the previous conversation where we decided to remove varm_layers)

I think we should land roughly here, to keep the code & schema modular:

• this function (get_anndata) should pass through all (or at least most) arguments supported by ExperimentAxisQuery.to_anndata. That makes it future-proof and decoupled from the schema
• the newly added args should have clearly separated names (the current names are not clear per above comment)
• the error checking needs to detect collisions because AnnData only has one "dict" to shove them all in, and they might conflict.

Boiling this down, I suggest:

1. Add (and pass to query.to_anndata) the args obsm_layers, varm_layers, obsp_layers, and varp_layers. These are just pass through.
2. Rename add_obs_embeddings and add_var_embeddings to something clear (see above discussion)
3. Do the error checks for collisions, and do them before you do any data loading. Example below.

Example error check for obs:

if set(obsm_layers) & set(add_obs_embeddings):
    ... there is a collision error ...

....do same for varm ...

Sounds good.

this whole blob could be a simple comprehension which might make it more pythonic (this is a nit, up to you). E.g,

return sorted({ obj['census_version'] for obj in manifest.values() if ...  })

And unless there are duplicates expected, I'm not sure what the set adds? If there are duplicates, doesn't that imply you need more filter criteria?

The set is because multiple embeddings can exist for a single alias, and in this case we're only interested in the census version string, so it needs to be deduplicated. I'll rewrite using the comprehension.

strongly prefer 1) and having a verbose argument